Skip to content

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented Aug 28, 2025

Description

This got a bit out of hand, but the main purpose of this PR is to fix a bug that caused the Feedback task to not correctly load the next task unless it was going to the archive/receipt. I added a gateway to ttd/stateless-app letting the test decide what happens after the feedback task, and you can choose for it to either fail (taking you back to try again) or continue (which will land you at the process end-event).

This uncovered a few things, which I'm fixing here:

  • When the feedback task went backwards to a previous task to try again, changing the radio button there failed. I noticed in the logs that app-frontend thought the data model was locked, but that's just because it failed to wait until process/next had finished fetching instance data again before using old instance data in DataModelsProvider to conclude the data element was locked (but it is unlocked when going backwards again). Fixing this by not providing DataModelsProvider when the instance data is fetching.
  • The router in App.tsx had a whole thing for CustomReceipt, but there's no real difference between CustomReceipt and any other data-task. Removing the duplication and adding <FixWrongReceiptType /> which seemed to be the only thing differentiating CustomReceipt from a normal form.
  • The PresentationComponent needed to know if you're in a receipt or not, but passing this in as a prop had gotten very complex. Instead of passing it in as a prop, the component itself can just useIsReceiptPage() to figure that out. Refactoring this, removing a prop.
  • useProcessQuery() had, until now, an effect that would navigate you to the receipt page if the process has ended. This is what actually did the magic in the Feedback task, but it was quite unclear that this was what broke navigating to any other task type after Feedback. Instead of just fixing things locally here, I re-engineered the whole thing. The Feedback component now does the navigation after polling find you at another task, and the effect to navigate to the correct receipt type was moved into FixWrongReceiptType instead. This way everyone can read the process query without applying any effects.
  • When working on different solutions, <ProcessWrapper /> got in the way quite a few times. When I tried making any generic "if the process data says we're on a different task now, navigate us there" functionality, it would get in the way of the existing "you're looking at a previous task" error page that could be rendered from <ProcessWrapper />. To make sure I didn't introduce any more bugs, I added Cypress tests (for ttd/stateless-app) that check:
    1. That navigating back to a previous task always shows you the error page, and lets you navigate to the current task by clicking the button. This test will become flaky if we introduce any code in the future that automatically navigates you, as Cypress will not always get to click the button in time - so this test also serves as a guard against making bugs I made during at least one attempt at implementing this.
    2. I also made one test fail if the error message showed up at any point during navigation. I've fixed this once before, but it's becoming a recurring theme that the error page appears very briefly during navigation to process/next, as there will be a tiny window where the URL has not caught up with the state in the process data query. Now a test will fail if we regress again.

Related Issue(s)

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* and backport* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

Summary by CodeRabbit

  • New Features

    • More consistent receipt experience: automatic redirects to the correct receipt, green receipt styling, and navigation/progress hidden on receipt pages.
    • Improved feedback flow reliability with clearer page title and more resilient retry/navigation behavior.
  • Bug Fixes

    • Removed transient “Denne delen av skjemaet er ikke tilgjengelig” warning during feedback.
    • Prevented navigation to incorrect receipt routes and fixed edge-case receipt routing.
  • Performance

    • Faster step transitions after submit via parallel data refresh.
  • Tests

    • Added end-to-end tests for stateless feedback and receipt flows.

@olemartinorg olemartinorg added kind/bug Something isn't working backport-ignore This PR is a new feature and should not be cherry-picked onto release branches labels Aug 28, 2025
Copy link

coderabbitai bot commented Aug 28, 2025

📝 Walkthrough

Walkthrough

Centralizes receipt detection with a new useIsReceiptPage hook and FixWrongReceiptType wrapper, removes the PresentationComponent type prop and type-driven branching, moves SearchParams to core/routing/types, adds navigation/query helpers (useIsNavigating, useWaitForQueries), converts some data-model shapes to tuples, and updates related imports, hooks, tests, and routes.

Changes

Cohort / File(s) Summary
Presentation API & call sites
src/components/presentation/Presentation.tsx, src/components/presentation/Presentation.test.tsx, src/core/loading/Loader.tsx, src/features/instantiate/selection/InstanceSelection.tsx, src/layout/Subform/SubformWrapper.tsx, src/features/pdf/PDFWrapper.test.tsx
Remove type prop from PresentationComponent and ProgressBar; replace type-driven UI branching with useIsReceiptPage; update tests and all call sites to omit type.
Routing & receipt flow
src/App.tsx, src/features/receipt/ReceiptContainer.tsx, src/features/receipt/FixWrongReceiptType.tsx, src/components/wrappers/ProcessWrapper.tsx
Remove CustomReceipt route/export, add FixWrongReceiptType component that conditionally redirects to correct receipt route, wrap routes with it, and adapt ProcessWrapper rendering to use new helpers and receipt gating.
SearchParams & navigation state
src/core/routing/types.ts, src/hooks/navigation.ts, src/core/routing/useIsNavigating.ts, src/core/routing/useIsReceiptPage.ts, src/hooks/useIsPdf.ts, src/hooks/useNavigatePage.ts, src/layout/GenericComponent.tsx, src/components/form/Form.tsx, src/components/form/LinkToPotentialNode.tsx, src/layout/RepeatingGroup/.../*.tsx, src/layout/Tabs/Tabs.tsx, src/features/navigation/AppNavigation.tsx, src/features/navigation/utils.ts, src/features/propagateTraceWhenPdf/index.ts
Move SearchParams enum out of hooks/navigation into core/routing/types; remove useIsReceiptPage from hooks/navigation; add standalone useIsNavigating and useIsReceiptPage hooks; update imports/usages and focus/navigation logic across many modules to the new locations.
Process navigation & queries
src/features/processEnd/feedback/Feedback.tsx, src/features/instance/useProcessQuery.ts, src/features/instance/useProcessNext.tsx, src/hooks/useWaitForQueries.ts
Rework feedback polling to callback-driven backoff with optimistic updates and navigation decisions; simplify useProcessQuery to return the raw process query; parallelize refetches in useProcessNext; add useWaitForQueries to await React Query activity.
Data model retrieval & utils
src/features/datamodel/DataModelsProvider.tsx, src/features/datamodel/utils.ts
Change instance data elements to tuple form [dataType, locked], adapt selection, mapping and isDataTypeWritable to the tuple shape, and update fetching logic to use useInstanceDataQuery.
Form-data selector lax variants & expressions
src/features/formData/FormDataWrite.tsx, src/layout/index.ts, src/utils/layout/useExpressionDataSources.ts, src/features/expressions/expression-functions.ts
Introduce FormDataSelectorLax and lax delayed/debounced selector exports; switch expression data sources to lax selectors; add guard for ContextNotProvided in expression functions.
E2E tests
test/e2e/integration/stateless-app/feedback.ts, test/e2e/integration/stateless-app/receipt.ts
Add end-to-end tests for stateless feedback and receipt navigation, including transient forbidden-message detection and manual URL-navigation assertions.
Misc / utilities & minor call-site updates
src/features/propagateTraceWhenPdf/index.ts, src/layout/GenericComponent.tsx, src/layout/RepeatingGroup/..., src/layout/Tabs/Tabs.tsx, src/components/form/LinkToPotentialNode.tsx, src/hooks/useIsPdf.ts, src/hooks/useNavigatePage.ts
Update imports to new SearchParams location, replace navigation-state checks with useIsNavigating, and minor refactors to integrate new hooks/utilities.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The pull request includes API surface modifications in form data contexts—such as renaming useDelayedSelectorProps to useLaxDelayedSelectorProps, introducing FormDataSelectorLax, and updating useExpressionDataSources—that are not mentioned in the linked issue or PR objectives and do not pertain to the Feedback navigation bug, indicating out-of-scope changes. Consider extracting the form data API and type updates into a separate pull request to keep this bugfix focused solely on correcting Feedback step navigation.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the core bug fixed by this changeset, namely enabling the Feedback step to navigate to a non-archive data task instead of only handling EndEvent receipts, without extraneous detail or noise.
Linked Issues Check ✅ Passed The changes directly address the core failure described in issue #3614 by refactoring the Feedback component to perform gateway-driven navigation for non-EndEvent tasks, moving receipt navigation logic to FixWrongReceiptType, updating useProcessQuery to remove automatic receipt routing, and adding DataModelsProvider guards to prevent stale data locking, thereby fulfilling the expected behavior of progressing to the task indicated by the process.
Description Check ✅ Passed The pull request description follows the repository template by clearly summarizing the change, linking the related issue (#3614), and providing detailed Verification/QA steps with manual and automated test checkboxes, accessibility notes, documentation and support considerations, sprint board updates, and label usage.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bug/feedback-step

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3d6e69 and 2c90305.

📒 Files selected for processing (3)
  • src/features/datamodel/DataModelsProvider.tsx (5 hunks)
  • src/features/instantiate/selection/InstanceSelection.tsx (1 hunks)
  • src/layout/Subform/SubformWrapper.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/layout/Subform/SubformWrapper.tsx
  • src/features/instantiate/selection/InstanceSelection.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options via queryOptions

Files:

  • src/features/datamodel/DataModelsProvider.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/receipt/FixWrongReceiptType.tsx:16-34
Timestamp: 2025-09-03T14:23:33.606Z
Learning: In FixWrongReceiptType.tsx, brief ping-ponging between receipt routes during instance data loading is acceptable because the instance data is typically already fetched when this component runs. The author considers this a non-issue.
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.

Applied to files:

  • src/features/datamodel/DataModelsProvider.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In the Altinn app-frontend-react codebase, "stateless" refers to "instanceless" applications. While stateless/instanceless apps don't have server-side instance data, they still require data models to be loaded and processed for local state management.

Applied to files:

  • src/features/datamodel/DataModelsProvider.tsx
🧬 Code graph analysis (1)
src/features/datamodel/DataModelsProvider.tsx (2)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (110-119)
src/features/datamodel/utils.ts (2)
  • MissingDataElementException (27-36)
  • isDataTypeWritable (104-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (7)
src/features/datamodel/DataModelsProvider.tsx (7)

4-4: LGTM: Import aligns with TanStack Query v5

The useMutationState import is used correctly at line 246 to detect pending save mutations.


30-30: LGTM: Imports support new data retrieval pattern

The hooks useInstanceDataElements and useInstanceDataQuery correctly replace the previous instance query mechanism.


160-163: LGTM: Query follows TanStack Query v5 patterns

The useInstanceDataQuery correctly:

  • Disables fetching for stateless apps
  • Transforms instance data to tuple shape [dataType, locked] via select
  • Uses object-based query options per coding guidelines

174-177: Verified: isFetching guard appropriate for stateless apps

This guard correctly prevents the effect from running before data is ready. As confirmed in previous discussion, stateless (instanceless) apps don't fetch instance data but still need this guard to work correctly with the effect's data processing logic.


197-201: LGTM: Tuple destructuring correctly accesses dataType

The tuple shape [dataType, locked] is correctly destructured to check for the existence of a data element of the given type.


205-207: LGTM: Fallback handles undefined dataElements

The dataElements ?? [] fallback correctly provides an empty array when data hasn't loaded yet, matching the expected tuple array type.


211-211: LGTM: Effect dependencies complete

The dependency array correctly includes dataElements and isFetching to ensure the effect re-runs when data fetch state changes.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
src/features/propagateTraceWhenPdf/index.ts (2)

6-13: Fix cookie parsing typing and edge-cases.
Current code creates a {} and assigns dynamic keys (unsafe) and may create empty keys.

-function getCookies(): { [key: string]: string } {
-  const cookie = {};
-  document.cookie.split(';').forEach((el) => {
-    const split = el.split('=');
-    cookie[split[0].trim()] = split.slice(1).join('=');
-  });
-  return cookie;
-}
+function getCookies(): Record<string, string> {
+  const cookies: Record<string, string> = {};
+  if (!document.cookie) return cookies;
+  document.cookie.split(';').forEach((raw) => {
+    const el = raw.trim();
+    if (!el) return;
+    const idx = el.indexOf('=');
+    if (idx === -1) return;
+    const key = el.slice(0, idx).trim();
+    const value = el.slice(idx + 1);
+    if (!key) return;
+    cookies[key] = decodeURIComponent(value);
+  });
+  return cookies;
+}

21-46: Avoid multiple interceptor registrations.
If propagateTraceWhenPdf() runs more than once, interceptors stack up.

-    if (isPdf) {
-      const cookies = getCookies();
-      axios.interceptors.request.use((config) => {
+    if (isPdf) {
+      const cookies = getCookies();
+      if (pdfInterceptorId != null) return;
+      pdfInterceptorId = axios.interceptors.request.use((config) => {
         try {
           if (config.url?.startsWith(appPath) !== true) {
             return config;
           }
 
           // This header is caught in app-lib/backend and used to allow injection of traceparent/context
-          config.headers['X-Altinn-IsPdf'] = 'true';
+          (config.headers ??= {})['X-Altinn-IsPdf'] = 'true';
 
           const traceparent = cookies['altinn-telemetry-traceparent'];
           const tracestate = cookies['altinn-telemetry-tracestate'];
           if (traceparent) {
-            config.headers['traceparent'] = traceparent;
+            (config.headers ??= {})['traceparent'] = traceparent;
           }
           if (tracestate) {
-            config.headers['tracestate'] = tracestate;
+            (config.headers ??= {})['tracestate'] = tracestate;
           }
           return config;
         } catch (err) {
           console.error('Error configuring propagation of W3C trace for request', err);
           return config;
         }
       });
     }

Additional top-level addition (outside the selected range):

// At module scope
let pdfInterceptorId: number | null = null;
src/features/instance/useProcessQuery.ts (1)

66-71: Bug: wrong task lookup uses currentTask regardless of match.
You ignore the found task in processTasks and always return currentTask, which misclassifies other tasks.

-  return (taskId: string | undefined) => {
-    const task =
-      (processData?.processTasks?.find((t) => t.elementId === taskId) ?? processData?.currentTask?.elementId === taskId)
-        ? processData?.currentTask
-        : undefined;
+  return (taskId: string | undefined) => {
+    const task =
+      processData?.processTasks?.find((t) => t.elementId === taskId) ??
+      (processData?.currentTask?.elementId === taskId ? processData?.currentTask : undefined);
src/components/form/Form.tsx (1)

53-56: Bug: search param not removed due to returning stale object

You mutate params but return the outer searchParams, so ?validate persists and can cause repeated validation/replace loops. Return a new instance (and prefer replace).

Apply:

-      setSearchParams((params) => {
-        params.delete(SearchParams.Validate);
-        return searchParams;
-      });
+      setSearchParams((prev) => {
+        const next = new URLSearchParams(prev);
+        next.delete(SearchParams.Validate);
+        return next;
+      }, { replace: true });
src/hooks/useNavigatePage.ts (1)

248-252: Bug: URLSearchParams.has does not accept a value argument

has(name, 'true') is invalid and always checks only presence. You likely want to check for the value being 'true'.

Apply this diff:

-      const shouldExitSubform = options?.searchParams?.has(SearchParams.ExitSubform, 'true') ?? false;
+      const shouldExitSubform = options?.searchParams?.get(SearchParams.ExitSubform) === 'true';
src/features/processEnd/feedback/Feedback.tsx (1)

60-71: Prevent overlapping refetches and handle errors in backoff loop

Currently, the next tick is scheduled immediately after triggering callback(), which can cause overlapping requests if a fetch takes longer than the backoff delay. Also, unhandled promise rejections can leak to the console. Await the callback (with catch) before scheduling the next attempt.

-      setTimeout(() => {
-        if (!shouldContinue) {
-          return;
-        }
-        callback().then();
-        attempts.current++;
-        shouldContinue && continueCalling();
-      }, backoff);
+      setTimeout(() => {
+        if (!shouldContinue) {
+          return;
+        }
+        Promise.resolve()
+          .then(() => callback())
+          .catch((err) => {
+            // Log once per failure; consider using a dedicated logger if available
+            console.error('Feedback polling failed', err);
+          })
+          .finally(() => {
+            attempts.current++;
+            if (shouldContinue) {
+              continueCalling();
+            }
+          });
+      }, backoff);
🧹 Nitpick comments (18)
src/core/routing/types.ts (1)

1-7: Optional: use a const object + union type instead of a TS enum

This avoids enum runtime artifacts, plays nicer with ESM tree-shaking, and retains the same call-site API.

-export enum SearchParams {
-  FocusComponentId = 'focusComponentId',
-  FocusErrorBinding = 'focusErrorBinding',
-  ExitSubform = 'exitSubform',
-  Validate = 'validate',
-  Pdf = 'pdf',
-}
+export const SearchParams = {
+  FocusComponentId: 'focusComponentId',
+  FocusErrorBinding: 'focusErrorBinding',
+  ExitSubform: 'exitSubform',
+  Validate: 'validate',
+  Pdf: 'pdf',
+} as const;
+export type SearchParamKey = keyof typeof SearchParams;
src/components/form/LinkToPotentialNode.tsx (1)

21-23: Tighten search extraction for robustness.
Safer handling when no query string and clearer intent.

-  const searchParams = typeof to === 'string' ? to.split('?').at(1) : to.search;
+  const searchParams =
+    typeof to === 'string'
+      ? (to.includes('?') ? to.slice(to.indexOf('?') + 1) : '')
+      : (to.search ?? '');
test/e2e/integration/stateless-app/receipt.ts (2)

19-23: Make the manual back-navigation URL replace resilient.
Preserve query/hash and avoid brittle end-of-string assumptions.

-    cy.url().then((url) => cy.visit(url.replace(/\/Task_2\/1$/, '/Task_1')));
+    cy.location('href').then((href) =>
+      cy.visit(href.replace(/\/Task_2\/1(?=(?:\?|#|$))/, '/Task_1')),
+    );

55-63: Ditto for navigation away from receipt.
Use the same resilient pattern.

-    cy.url().then((url) => cy.visit(url.replace(/\/ProcessEnd$/, '/Task_4')));
+    cy.location('href').then((href) =>
+      cy.visit(href.replace(/\/ProcessEnd(?=(?:\?|#|$))/, '/Task_4')),
+    );
src/features/instance/useProcessNext.tsx (1)

44-47: Remove duplicate hook call; reuse one reference.
useOnFormSubmitValidation() is called twice under two names.

-  const onFormSubmitValidation = useOnFormSubmitValidation();
+  const runFormSubmitValidation = useOnFormSubmitValidation();
@@
-  const onSubmitFormValidation = useOnFormSubmitValidation();
@@
-      const hasErrors = await onFormSubmitValidation();
+      const hasErrors = await runFormSubmitValidation();
@@
-        const hasValidationErrors = await onSubmitFormValidation(true);
+        const hasValidationErrors = await runFormSubmitValidation(true);
src/features/datamodel/utils.ts (2)

104-108: Strengthen typing: explicit return type and tuple alias for readability

Give the function an explicit boolean return type and use a named, labeled tuple type instead of raw indices to avoid “magic numbers”.

-export function isDataTypeWritable(
-  dataType: string | undefined,
-  isStateless: boolean,
-  dataElements: (readonly [string, boolean])[],
-) {
+export function isDataTypeWritable(
+  dataType: string | undefined,
+  isStateless: boolean,
+  dataElements: readonly DataElementTuple[],
+): boolean {

Add this type once in the module (outside the selected range):

export type DataElementTuple = readonly [dataType: string, locked: boolean];

115-116: Minor clarity: handle undefined result explicitly

Avoid relying on coercion; return false when the element isn’t found, and keep the boolean semantics obvious.

-  return !!dataElement && !dataElement[1];
+  return dataElement ? !dataElement[1] : false;
src/features/datamodel/DataModelsProvider.tsx (3)

159-163: Prefer isSuccess over isFetching; tighten select typing to drop as const

Using isSuccess better models “data is ready” than isFetching (which also toggles during background refetches). Also, provide a concrete return type for select to avoid the as const assertion.

-  const { data: dataElements, isFetching } = useInstanceDataQuery({
+  const { data: dataElements, isSuccess } = useInstanceDataQuery<readonly DataElementTuple[]>({
     enabled: !isStateless,
-    select: (data) => data.data.map((element) => [element.dataType, element.locked] as const),
+    select: (data) => data.data.map(({ dataType, locked }) => [dataType, locked]) as const,
   });

(Assumes DataElementTuple from utils is exported; otherwise define a local alias.)


195-205: Avoid repeated linear scans by precomputing a Set

Micro-optimization and clearer intent when checking for presence across many data types.

-      if (!isStateless && !dataElements?.find(([dt]) => dt === dataType)) {
+      const types = new Set((dataElements ?? []).map(([dt]) => dt));
+      if (!isStateless && !types.has(dataType)) {
         const error = new MissingDataElementException(dataType);
         window.logErrorOnce(error.message);
         continue;
       }
@@
-      if (isDataTypeWritable(dataType, isStateless, dataElements ?? [])) {
+      if (isDataTypeWritable(dataType, isStateless, dataElements ?? [])) {
         writableDataTypes.push(dataType);
       }

Note: declare types once before the loop if you keep using it multiple times.


209-209: Keep dependencies in sync with control flow

If you adopt isSuccess, remove isFetching from the deps and add isSuccess. Also, dataElements can be omitted if you derive the Set inside the effect when isSuccess is true.

-  }, [applicationMetadata, defaultDataType, isStateless, layouts, setDataTypes, dataElements, layoutSetId, isFetching]);
+  }, [applicationMetadata, defaultDataType, isStateless, layouts, setDataTypes, layoutSetId, isSuccess]);
src/features/navigation/utils.ts (1)

22-27: Ensure boolean return value

pageGroups || taskGroups.length can yield a non-boolean (object or number). Cast to boolean to keep types predictable.

-  return !isReceiptPage && (pageGroups || taskGroups.length);
+  return !isReceiptPage && !!(pageGroups || taskGroups.length > 0);
src/components/form/Form.tsx (1)

239-258: Remove unused dependency and variable

onFormSubmitValidation is declared but not used; also listed in deps causing needless effect runs.

-  const onFormSubmitValidation = useOnFormSubmitValidation();
   const exitSubform = useQueryKey(SearchParams.ExitSubform)?.toLocaleLowerCase() === 'true';
   const validate = useQueryKey(SearchParams.Validate)?.toLocaleLowerCase() === 'true';
   const navigate = useNavigate();
   const searchStringRef = useAsRef(useLocation().search);
@@
-  }, [navigate, searchStringRef, exitSubform, validate, onFormSubmitValidation]);
+  }, [navigate, searchStringRef, exitSubform, validate]);
src/core/routing/useIsReceiptPage.ts (1)

4-7: Receipt detection hook looks good; minor readability tweak optional

If the list grows, a Set reads cleaner.

-  return taskId === TaskKeys.CustomReceipt || taskId === TaskKeys.ProcessEnd;
+  return new Set([TaskKeys.CustomReceipt, TaskKeys.ProcessEnd]).has(taskId);
src/hooks/useNavigatePage.ts (1)

235-240: Avoid emitting trailing “?” when no search params

When stateless, the URL can end with a bare “?” if there are no params.

Apply this diff:

-    if (isStatelessApp && orderRef.current[0] !== undefined && (!currentPageId || !isValidPageId(currentPageId))) {
-      navigate(`/${orderRef.current[0]}?${searchParamsRef.current}`, { replace: true });
-    }
+    if (isStatelessApp && orderRef.current[0] !== undefined && (!currentPageId || !isValidPageId(currentPageId))) {
+      const q = searchParamsRef.current.toString();
+      const qs = q ? `?${q}` : '';
+      navigate(`/${orderRef.current[0]}${qs}`, { replace: true });
+    }
src/features/receipt/FixWrongReceiptType.tsx (1)

21-26: Prefer const expression for redirect target

Minor readability: compute redirectTo as a single const expression.

Apply this diff:

-  let redirectTo: undefined | TaskKeys = undefined;
-  if (taskId === TaskKeys.ProcessEnd && hasCustomReceipt) {
-    redirectTo = TaskKeys.CustomReceipt;
-  } else if (taskId === TaskKeys.CustomReceipt && !hasCustomReceipt) {
-    redirectTo = TaskKeys.ProcessEnd;
-  }
+  const redirectTo: TaskKeys | undefined =
+    taskId === TaskKeys.ProcessEnd && hasCustomReceipt
+      ? TaskKeys.CustomReceipt
+      : taskId === TaskKeys.CustomReceipt && !hasCustomReceipt
+        ? TaskKeys.ProcessEnd
+        : undefined;
src/layout/GenericComponent.tsx (1)

230-233: Non-standard scroll behavior value

scrollIntoView({ behavior: 'instant' }) is non-standard. Use 'auto' (or 'smooth') to avoid cross-browser issues.

-          !abortController.current.signal.aborted && div.scrollIntoView({ behavior: 'instant' });
+          !abortController.current.signal.aborted && div.scrollIntoView({ behavior: 'auto' });
src/components/presentation/Presentation.test.tsx (1)

91-97: Rename tests to match new hook-driven behavior (cosmetic)

Titles still reference ProcessTaskType; assertions are now driven by useIsReceiptPage. Rename for clarity.

Example:

  • “background is greyLight when not receipt page”
  • “background is greenLight when receipt page”

Also applies to: 99-107

src/features/processEnd/feedback/Feedback.tsx (1)

55-58: Comment/backoff mismatch

The current formula reaches the 30s max delay around attempt ≈ 39 (starting from 0), not 20. Either adjust the formula or update the comment to reflect reality.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a18a687 and d55fe41.

📒 Files selected for processing (33)
  • src/App.tsx (3 hunks)
  • src/components/form/Form.tsx (2 hunks)
  • src/components/form/LinkToPotentialNode.tsx (1 hunks)
  • src/components/presentation/Presentation.test.tsx (6 hunks)
  • src/components/presentation/Presentation.tsx (5 hunks)
  • src/components/wrappers/ProcessWrapper.tsx (3 hunks)
  • src/core/loading/Loader.tsx (0 hunks)
  • src/core/routing/types.ts (1 hunks)
  • src/core/routing/useIsNavigating.ts (1 hunks)
  • src/core/routing/useIsReceiptPage.ts (1 hunks)
  • src/features/datamodel/DataModelsProvider.tsx (4 hunks)
  • src/features/datamodel/utils.ts (1 hunks)
  • src/features/expressions/expression-functions.ts (1 hunks)
  • src/features/instance/useProcessNext.tsx (1 hunks)
  • src/features/instance/useProcessQuery.ts (2 hunks)
  • src/features/instantiate/selection/InstanceSelection.tsx (1 hunks)
  • src/features/navigation/AppNavigation.tsx (2 hunks)
  • src/features/navigation/utils.ts (1 hunks)
  • src/features/pdf/PDFWrapper.test.tsx (1 hunks)
  • src/features/processEnd/feedback/Feedback.tsx (3 hunks)
  • src/features/propagateTraceWhenPdf/index.ts (1 hunks)
  • src/features/receipt/FixWrongReceiptType.tsx (1 hunks)
  • src/features/receipt/ReceiptContainer.tsx (3 hunks)
  • src/hooks/navigation.ts (1 hunks)
  • src/hooks/useIsPdf.ts (1 hunks)
  • src/hooks/useNavigatePage.ts (2 hunks)
  • src/layout/GenericComponent.tsx (2 hunks)
  • src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx (1 hunks)
  • src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx (1 hunks)
  • src/layout/Subform/SubformWrapper.tsx (1 hunks)
  • src/layout/Tabs/Tabs.tsx (1 hunks)
  • test/e2e/integration/stateless-app/feedback.ts (1 hunks)
  • test/e2e/integration/stateless-app/receipt.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • src/core/loading/Loader.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options via queryOptions

Files:

  • src/features/propagateTraceWhenPdf/index.ts
  • src/features/pdf/PDFWrapper.test.tsx
  • src/features/instance/useProcessNext.tsx
  • src/layout/Subform/SubformWrapper.tsx
  • src/features/instantiate/selection/InstanceSelection.tsx
  • src/features/receipt/FixWrongReceiptType.tsx
  • src/core/routing/useIsReceiptPage.ts
  • src/hooks/useIsPdf.ts
  • src/layout/Tabs/Tabs.tsx
  • src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx
  • src/features/navigation/utils.ts
  • test/e2e/integration/stateless-app/receipt.ts
  • src/features/instance/useProcessQuery.ts
  • src/components/form/LinkToPotentialNode.tsx
  • src/features/processEnd/feedback/Feedback.tsx
  • src/core/routing/types.ts
  • src/core/routing/useIsNavigating.ts
  • test/e2e/integration/stateless-app/feedback.ts
  • src/components/presentation/Presentation.test.tsx
  • src/components/form/Form.tsx
  • src/features/expressions/expression-functions.ts
  • src/features/receipt/ReceiptContainer.tsx
  • src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx
  • src/components/presentation/Presentation.tsx
  • src/features/datamodel/DataModelsProvider.tsx
  • src/layout/GenericComponent.tsx
  • src/features/datamodel/utils.ts
  • src/hooks/navigation.ts
  • src/components/wrappers/ProcessWrapper.tsx
  • src/App.tsx
  • src/features/navigation/AppNavigation.tsx
  • src/hooks/useNavigatePage.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

In tests, use renderWithProviders from src/test/renderWithProviders.tsx to supply required form layout context

Files:

  • src/features/pdf/PDFWrapper.test.tsx
  • src/components/presentation/Presentation.test.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.{ts,tsx} : For TanStack Query, use objects to manage query keys and functions, and centralize shared options via `queryOptions`

Applied to files:

  • src/features/instance/useProcessQuery.ts
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context

Applied to files:

  • src/components/presentation/Presentation.test.tsx
  • src/features/datamodel/DataModelsProvider.tsx
🧬 Code graph analysis (12)
src/features/pdf/PDFWrapper.test.tsx (1)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/layout/Subform/SubformWrapper.tsx (1)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/instantiate/selection/InstanceSelection.tsx (1)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/receipt/FixWrongReceiptType.tsx (5)
src/hooks/navigation.ts (1)
  • useNavigationParam (51-55)
src/features/form/layoutSets/LayoutSetsProvider.tsx (1)
  • useLayoutSets (52-52)
src/utils/formLayout.ts (1)
  • behavesLikeDataTask (8-14)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/core/loading/Loader.tsx (1)
  • Loader (14-36)
src/core/routing/useIsReceiptPage.ts (1)
src/hooks/navigation.ts (1)
  • useNavigationParam (51-55)
src/features/processEnd/feedback/Feedback.tsx (3)
src/features/instance/useProcessQuery.ts (2)
  • useProcessQuery (23-26)
  • useOptimisticallyUpdateProcess (99-106)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
test/e2e/integration/stateless-app/feedback.ts (1)
test/e2e/pageobjects/app-frontend.ts (1)
  • AppFrontend (3-371)
src/features/receipt/ReceiptContainer.tsx (3)
src/features/receipt/FixWrongReceiptType.tsx (1)
  • FixWrongReceiptType (15-39)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/form/FormContext.tsx (1)
  • FormProvider (43-86)
src/components/presentation/Presentation.tsx (7)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
src/features/form/layout/UiConfigContext.tsx (1)
  • useUiConfigContext (44-44)
src/features/navigation/utils.ts (1)
  • useHasGroupedNavigation (22-27)
src/core/routing/useIsReceiptPage.ts (1)
  • useIsReceiptPage (4-7)
src/features/language/Lang.tsx (1)
  • Lang (15-23)
src/theme/altinnAppTheme.tsx (1)
  • AltinnPalette (2-25)
src/features/form/layoutSettings/LayoutSettingsContext.tsx (1)
  • usePageSettings (131-140)
src/features/datamodel/DataModelsProvider.tsx (2)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
src/features/datamodel/utils.ts (2)
  • MissingDataElementException (27-36)
  • isDataTypeWritable (104-117)
src/layout/GenericComponent.tsx (1)
src/core/routing/useIsNavigating.ts (1)
  • useIsNavigating (3-7)
src/components/wrappers/ProcessWrapper.tsx (6)
src/hooks/useNavigatePage.ts (1)
  • useIsValidTaskId (184-202)
src/features/instance/useProcessQuery.ts (2)
  • useGetTaskTypeById (61-89)
  • useProcessQuery (23-26)
src/features/form/FormContext.tsx (1)
  • FormProvider (43-86)
src/features/receipt/ReceiptContainer.tsx (1)
  • CustomReceipt (92-112)
src/features/instance/useProcessNext.tsx (1)
  • getProcessNextMutationKey (30-35)
src/core/routing/useIsNavigating.ts (1)
  • useIsNavigating (3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Install
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (34)
src/core/routing/types.ts (2)

1-7: Centralized SearchParams: LGTM

Good move consolidating routing search keys into a single exported source.


1-7: All SearchParams imports have been migrated
No occurrences of import { SearchParams } from 'src/hooks/navigation' remain.

src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx (1)

6-6: Import path switched to core/routing: LGTM

Consistent with the centralization of SearchParams.

src/features/expressions/expression-functions.ts (1)

4-4: Import path switched to core/routing: LGTM

No behavior change; linkToComponent still builds URLs with FocusComponentId as before.

src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx (1)

6-6: Import path switched to core/routing: LGTM

Matches the new single source of truth for search params.

src/layout/Tabs/Tabs.tsx (1)

7-7: Import path switched to core/routing: LGTM

Keeps tab auto-activation behavior intact when focusing a component via URL.

src/components/form/LinkToPotentialNode.tsx (1)

5-5: Import path migration looks correct.
Keeps routing params centralized in core/routing.

src/features/propagateTraceWhenPdf/index.ts (1)

3-3: Import path migration looks correct.
Aligns with new routing/types location.

src/features/instance/useProcessQuery.ts (1)

25-26: Good: queryOptions + useQuery pattern.
Matches our TanStack Query guideline for centralized keys/options.

src/features/instance/useProcessNext.tsx (1)

92-92: Nice: parallel refetch cuts latency.
Refetching process and instance in parallel is a solid improvement.

src/features/datamodel/DataModelsProvider.tsx (1)

293-295: Hook return shapes unchanged
useInstanceDataElements still returns IData[] (objects with .id and .dataType), and getFirstDataElementId continues to accept that array—no downstream updates required.

src/features/pdf/PDFWrapper.test.tsx (1)

56-59: LGTM: aligns test with PresentationComponent API

Removing the obsolete type prop matches the new receipt-driven Presentation logic.

src/features/navigation/utils.ts (1)

13-13: LGTM: updated import path

Importing useIsReceiptPage from the new routing location is consistent with the refactor.

src/features/instantiate/selection/InstanceSelection.tsx (2)

48-51: LGTM: remove deprecated type prop

Presentation is now receipt-aware; dropping the type prop here is correct.


1-340: Audit ProcessTaskType usage; PresentationComponent check passed

  • No lingering type= props found on PresentationComponent.
  • ProcessTaskType is still referenced in multiple files (e.g., src/types/index.ts, src/hooks/useNavigatePage.ts, src/layout/SigneeList/index.tsx, etc.)—verify and remove or refactor any obsolete occurrences.
src/components/form/Form.tsx (2)

10-10: Centralize SearchParams import — OK

Import moved to core routing. Consistent with the refactor.


47-48: Clarify allowed values for validate param

Here any truthy value triggers validation, while elsewhere you strictly compare to 'true'. Align semantics or document accepted values ('true' vs '1'/presence).

src/hooks/useIsPdf.ts (1)

1-2: Import path update — OK

SearchParams now comes from core routing; hook behavior unchanged.

src/layout/Subform/SubformWrapper.tsx (1)

26-29: Updated PresentationComponent usage — OK

Removal of the type prop matches the new Presentation API.

src/features/navigation/AppNavigation.tsx (2)

7-7: useIsReceiptPage import path update — OK

Aligned with new core routing hook location; no logic change.


16-16: Retained useIsSubformPage from hooks — OK

Split sources are consistent.

src/hooks/useNavigatePage.ts (2)

5-5: Import move looks good

Importing SearchParams from core routing types is consistent with the refactor.


165-171: Receipt remapping in task navigation is solid

Good normalization of receipt destinations to the actual configured receipt type.

src/features/receipt/ReceiptContainer.tsx (2)

84-88: Good: Centralized receipt-type correction wrapper

Wrapping DefaultReceipt with FixWrongReceiptType ensures correct redirection without duplicating route logic.


106-111: Good: Custom receipt guarded and wrapped

Guarding missing data model and using the wrapper keeps navigation consistent while avoiding redirect loops.

src/App.tsx (5)

13-13: Route import simplification is correct

Using only DefaultReceipt aligns with the wrapper-based redirection.


36-36: Removal of type-prop from PresentationComponent is consistent

Matches the move to receipt-page detection.


61-63: Receipt route wiring looks correct

ProcessEnd now renders DefaultReceipt (with fix wrapper), simplifying routing.


84-86: Presentation without type-prop in PDF flow is consistent

Good alignment with Presentation changes.


60-74: Verify deep links to CustomReceipt still work as intended

Since CustomReceipt has no dedicated route, confirm that deep links to /instance/:partyId/:guid/CustomReceipt still render correctly (custom layout) or are redirected appropriately via FixWrongReceiptType or process logic.

Steps:

  • In an app with a custom receipt, open an instance at /ProcessEnd and ensure redirect to custom receipt occurs.
  • In an app without a custom receipt, navigate to /CustomReceipt and verify redirect/fallback to /ProcessEnd.
  • Confirm browser history contains a single entry after redirects (replace = true).
src/hooks/navigation.ts (1)

4-5: Good: centralizing SearchParams typing improves safety

Switching useQueryKey to accept the shared SearchParams type removes stringly-typed keys and prevents typos. LGTM.

Also applies to: 60-61

src/components/presentation/Presentation.tsx (3)

43-47: Receipt-aware presentation is correct

Using useIsReceiptPage() to select header, background, and hide navigation on receipts is consistent with the new routing model.


73-78: Substatus only on receipts

Conditionally rendering AltinnSubstatus only for receipts (and when a substatus exists) avoids confusing intermediate states.


84-85: Progress bar placement and simplification look good

Moving the ProgressBar into Header and rendering only when showProgress is true reads well and avoids special-casing in receipt views.

Also applies to: 95-99

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/presentation/Presentation.test.tsx (1)

111-116: Remove legacy type prop and import; they’re no longer part of the API

Tests still pass a type prop and import ProcessTaskType, which conflicts with the stated refactor.

Apply this diff:

-import { ProcessTaskType } from 'src/types';
@@
   const allProps = {
     header: 'Header text',
-    type: ProcessTaskType.Unknown,
     ...props,
   };
♻️ Duplicate comments (5)
src/components/presentation/Presentation.test.tsx (1)

32-35: Restore property spies to avoid cross-test bleed — addressed

Adding jest.restoreAllMocks() fixes the teardown gap flagged earlier. Nice.

src/features/processEnd/feedback/Feedback.tsx (2)

24-31: LGTM: ended-first check and data-task navigation

Prioritizing ended before task change prevents transient misnavigation and enables routing to non-EndEvent tasks (e.g., data tasks). This addresses the core bug.


13-14: Fix stale previousData capture and avoid unnecessary backoff resets

The callback closes over previousData, and including it in the dependency array restarts polling frequently. Track the last task id in a ref and compare against that inside a stable callback.

-  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const previousTaskRef = useRef<string | undefined>(previousData?.currentTask?.elementId);
+  useEffect(() => {
+    previousTaskRef.current = previousData?.currentTask?.elementId;
+  }, [previousData?.currentTask?.elementId]);
@@
-  const callback = useCallback(async () => {
+  const callback = useCallback(async () => {
     const result = await reFetchProcessData();
     let navigateTo: undefined | string;
     if (result.data?.ended) {
       navigateTo = TaskKeys.ProcessEnd;
     } else if (
       result.data?.currentTask?.elementId &&
-      result.data.currentTask.elementId !== previousData?.currentTask?.elementId
+      result.data.currentTask.elementId !== previousTaskRef.current
     ) {
       navigateTo = result.data.currentTask.elementId;
     }
 
     if (navigateTo && result.data) {
       optimisticallyUpdateProcess(result.data);
       await reFetchInstanceData();
       navigateToTask(navigateTo);
     }
-  }, [
-    navigateToTask,
-    optimisticallyUpdateProcess,
-    previousData?.currentTask?.elementId,
-    reFetchInstanceData,
-    reFetchProcessData,
-  ]);
+  }, [navigateToTask, optimisticallyUpdateProcess, reFetchInstanceData, reFetchProcessData]);
@@
-  useBackoff(callback);
+  useBackoff(callback);

Also applies to: 21-44, 47-47

src/components/wrappers/ProcessWrapper.tsx (2)

71-94: Make process/next gating reactive with useIsMutating

Using queryClient.isMutating snapshots once and may miss updates; prefer useIsMutating.

Apply this diff:

@@
-  const processNextKey = getProcessNextMutationKey();
-  const queryClient = useQueryClient();
-  const isRunningProcessNext = queryClient.isMutating({ mutationKey: processNextKey });
+  const processNextKey = getProcessNextMutationKey();
+  const isRunningProcessNext = useIsMutating({ mutationKey: processNextKey }) > 0;

And below:

@@
-import { useQueryClient } from '@tanstack/react-query';
-import type { QueryClient } from '@tanstack/react-query';
+import { useIsMutating, useQueryClient } from '@tanstack/react-query';
+import type { QueryClient } from '@tanstack/react-query';

181-193: Simplify and modernize useIsRunningProcessNext

Replace stateful snapshot with useIsMutating to avoid stale state.

Apply this diff:

-function useIsRunningProcessNext() {
-  const queryClient = useQueryClient();
-  const [isMutating, setIsMutating] = useState<boolean | null>(null);
-
-  // Intentionally wrapped in a useEffect() and saved as a state. If this happens, we'll seemingly be locked out
-  // with a <Loader /> forever, but when this happens, we also know we'll be re-rendered soon. This is only meant to
-  // block rendering when we're calling process/next.
-  useEffect(() => {
-    setIsMutating(isRunningProcessNext(queryClient));
-  }, [queryClient]);
-
-  return isMutating;
-}
+function useIsRunningProcessNext() {
+  const count = useIsMutating({ mutationKey: getProcessNextMutationKey() });
+  return count > 0;
+}
🧹 Nitpick comments (9)
src/components/presentation/Presentation.test.tsx (3)

37-50: Assert the query function is actually used

Optional: wrap fetchReturnUrl in a mock and assert it was called; improves test intent and guards refactors.

Apply this diff within this test:

   it('should link to query parameter returnUrl if valid URL', async () => {
     const returnUrl = 'foo';

     const mockedLocation = { ...realLocation, search: `?returnUrl=${returnUrl}` };
     jest.spyOn(window, 'location', 'get').mockReturnValue(mockedLocation);

-    await render({}, { fetchReturnUrl: async () => returnUrl });
+    const fetchReturnUrlMock = jest.fn().mockResolvedValue(returnUrl);
+    await render({}, { fetchReturnUrl: fetchReturnUrlMock });

     const closeButton = screen.getByRole('link', {
       name: 'Tilbake',
     });

     expect(closeButton).toHaveAttribute('href', returnUrl);
+    expect(fetchReturnUrlMock).toHaveBeenCalled();
   });

92-98: Rename test to reflect hook semantics, not ProcessTaskType.Data

The component no longer uses a type prop; the name should express isReceiptPage=false.

Apply this diff:

-  it('the background color should be greyLight if type is "ProcessTaskType.Data"', async () => {
+  it('the background color should be greyLight when isReceiptPage=false', async () => {

100-108: Tighten semantics and scope the mock to this test

  • Rename to reflect isReceiptPage=true.
  • Prefer mockReturnValueOnce(true) for tighter scope.

Apply this diff:

-  it('the background color should be lightGreen if type is "ProcessTaskType.Archived"', async () => {
-    mockUseIsReceiptPage.mockReturnValue(true);
+  it('the background color should be lightGreen when isReceiptPage=true', async () => {
+    mockUseIsReceiptPage.mockReturnValueOnce(true);
src/features/instance/useProcessNext.tsx (1)

92-93: Parallel refetch is right; consider allSettled to avoid aborting on a single failure

If either refetch rejects, Promise.all will reject and skip invalidation/navigation work. Using allSettled is a low-risk hardening.

-        await Promise.all([refetchProcessData(), reFetchInstanceData()]);
+        await Promise.allSettled([refetchProcessData(), reFetchInstanceData()]);
src/features/processEnd/feedback/Feedback.tsx (1)

59-85: Backoff comments don’t match the math; also clear timers on unmount

  • The code reaches 30s delay near attempt ~40, not 20. Fix the comment for accuracy.
  • Clear the timeout on cleanup to avoid stray queued callbacks; you already guard with shouldContinue, but clearing is cleaner.
 function useBackoff(callback: () => Promise<void>) {
   // The backoff algorithm is used to check the process data, and slow down the requests after a while.
   // At first, it starts off once a second (every 1000ms) for 10 seconds.
-  // After that, it slows down by one more second for every request.
-  // Once it reaches 20 attempts, it will reach the max delay of 30 seconds between each request.
+  // After that, it slows down by one more second for every request.
+  // It reaches the max delay of 30 seconds between each request after ~40 attempts with the current formula.
   const attempts = useRef(0);
+  const timeoutRef = useRef<number | undefined>(undefined);
 
   useEffect(() => {
     let shouldContinue = true;
     function continueCalling() {
       const backoff = attempts.current < 10 ? 1000 : Math.min(30000, 1000 + (attempts.current - 10) * 1000);
-      setTimeout(() => {
+      timeoutRef.current = window.setTimeout(() => {
         if (!shouldContinue) {
           return;
         }
         callback().then();
         attempts.current++;
         shouldContinue && continueCalling();
       }, backoff);
     }
 
     continueCalling();
 
     return () => {
       shouldContinue = false;
+      if (timeoutRef.current !== undefined) {
+        clearTimeout(timeoutRef.current);
+      }
     };
   }, [callback]);
 }
src/features/receipt/FixWrongReceiptType.tsx (1)

29-33: Harden logging

window.logWarnOnce may be undefined in some environments; guard it and fall back to console.warn.

See the logging changes included in the diff above.

src/features/receipt/ReceiptContainer.tsx (1)

101-110: Avoid duplicate useLanguage calls and use the existing langTools

Minor cleanup: you call useLanguage() twice; reuse langTools for langAsString.

Apply this diff:

@@
-  const langTools = useLanguage();
+  const langTools = useLanguage();
@@
-  const { langAsString } = useLanguage();
@@
-      <title>{`${getPageTitle(appName, langAsString('receipt.title'), appOwner)}`}</title>
+      <title>{`${getPageTitle(appName, langTools.langAsString('receipt.title'), appOwner)}`}</title>

Also applies to: 176-179

src/components/wrappers/ProcessWrapper.tsx (2)

195-228: Return a strict boolean from useIsWrongTask

Ensure the hook returns true|false, not null or a truthy value; avoids ambiguous checks.

Apply this diff:

-  return isWrongTask && !isCurrentTask && !isNavigating;
+  return isWrongTask === true && !isCurrentTask && !isNavigating;

120-126: Optional: Prefer auto-navigation to current task over showing an error on Feedback/Confirm

To better satisfy “Feedback step should navigate to a data task,” render Feedback/Confirm even if isWrongTask just flipped and let them drive navigation; move the isWrongTask guard below these branches or auto-navigate when wrong.

Proposed reordering:

-  if (isWrongTask) {
-    return (
-      <PresentationComponent showNavigation={false}>
-        <NavigationError label='general.part_of_form_completed' />
-      </PresentationComponent>
-    );
-  }
@@
   if (taskType === ProcessTaskType.Feedback) {
     return (
       <PresentationComponent>
         <Feedback />
       </PresentationComponent>
     );
   }
+
+  if (isWrongTask) {
+    return (
+      <PresentationComponent showNavigation={false}>
+        <NavigationError label='general.part_of_form_completed' />
+      </PresentationComponent>
+    );
+  }

Also applies to: 136-142

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c417bae and 9ca4439.

📒 Files selected for processing (7)
  • src/App.tsx (4 hunks)
  • src/components/presentation/Presentation.test.tsx (6 hunks)
  • src/components/wrappers/ProcessWrapper.tsx (3 hunks)
  • src/features/instance/useProcessNext.tsx (2 hunks)
  • src/features/processEnd/feedback/Feedback.tsx (3 hunks)
  • src/features/receipt/FixWrongReceiptType.tsx (1 hunks)
  • src/features/receipt/ReceiptContainer.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options via queryOptions

Files:

  • src/features/instance/useProcessNext.tsx
  • src/App.tsx
  • src/features/receipt/FixWrongReceiptType.tsx
  • src/components/wrappers/ProcessWrapper.tsx
  • src/features/receipt/ReceiptContainer.tsx
  • src/components/presentation/Presentation.test.tsx
  • src/features/processEnd/feedback/Feedback.tsx
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

In tests, use renderWithProviders from src/test/renderWithProviders.tsx to supply required form layout context

Files:

  • src/components/presentation/Presentation.test.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-28T12:00:31.684Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:31.684Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.

Applied to files:

  • src/features/instance/useProcessNext.tsx
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context

Applied to files:

  • src/components/presentation/Presentation.test.tsx
🧬 Code graph analysis (4)
src/App.tsx (3)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/receipt/FixWrongReceiptType.tsx (1)
  • FixWrongReceiptType (16-47)
src/features/form/FormContext.tsx (1)
  • FormProvider (43-86)
src/features/receipt/FixWrongReceiptType.tsx (6)
src/hooks/navigation.ts (1)
  • useNavigationParam (51-55)
src/features/form/layoutSets/LayoutSetsProvider.tsx (1)
  • useLayoutSets (52-52)
src/utils/formLayout.ts (1)
  • behavesLikeDataTask (8-14)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/features/datamodel/useBindingSchema.tsx (1)
  • useCurrentDataModelGuid (32-50)
src/core/loading/Loader.tsx (1)
  • Loader (14-36)
src/features/receipt/ReceiptContainer.tsx (2)
src/features/receipt/FixWrongReceiptType.tsx (1)
  • FixWrongReceiptType (16-47)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/processEnd/feedback/Feedback.tsx (3)
src/features/instance/useProcessQuery.ts (2)
  • useProcessQuery (23-26)
  • useOptimisticallyUpdateProcess (99-106)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Install
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
src/components/presentation/Presentation.test.tsx (5)

8-8: Switch to receipt-page hook aligns tests with new routing logic

Importing and relying on useIsReceiptPage is the right pivot away from the legacy type prop.


18-23: Mock is set up correctly and typed

Good use of a module factory and jest.MockedFunction for useIsReceiptPage.


27-31: Defaulting isReceiptPage to false in beforeEach prevents leakage

Solid baseline for each test.


59-60: Hook-driven default rendering path is correct

Rendering with no explicit type prop matches the new behavior.


75-76: Good: default path when returnUrl is missing

Keeps behavior consistent and readable.

src/features/instance/useProcessNext.tsx (1)

38-38: Good: lazy instance refetch prevents extra observer

Switching to useInstanceDataQuery({ enabled: false }).refetch avoids an unnecessary query observer and matches our prior learning for stateless vs. non-stateless flows.

src/features/processEnd/feedback/Feedback.tsx (1)

19-20: Good: disable instance query and use only refetch

useInstanceDataQuery({ enabled: false }).refetch avoids an extra live observer on the feedback page while still letting you pull when needed.

src/features/receipt/ReceiptContainer.tsx (1)

76-83: Good use of wrapper to normalize receipt routing

Wrapping DefaultReceipt with FixWrongReceiptType ensures correct target receipt without duplicating routing concerns elsewhere.

src/App.tsx (2)

61-65: Explicit ProcessEnd route rendering via DefaultReceipt looks correct

Directly routing ProcessEnd to DefaultReceipt aligns with the new receipt normalization.


66-76: Receipt guard applied to entire task flow is appropriate

Wrapping the task route with FixWrongReceiptType centralizes receipt redirection.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/features/processEnd/feedback/Feedback.tsx (1)

63-90: Prevent overlapping polls and handle errors; align backoff math with comments

  • The current loop fires the next timeout without awaiting callback(), so polls can overlap and spike load; re-schedule only after the promise settles.
  • Swallow rejections to keep polling alive.
  • Fix the off-by-one in backoff (attempt 10 should move to 2s) and update comments to reflect actual behavior.
 function useBackoff(callback: () => Promise<void>) {
-  // The backoff algorithm is used to check the process data, and slow down the requests after a while.
-  // At first, it starts off once a second (every 1000ms) for 10 seconds.
-  // After that, it slows down by one more second for every request.
-  // Once it reaches 20 attempts, it will reach the max delay of 30 seconds between each request.
+  // Poll with increasing delay to reduce load over time.
+  // - 1s delay for the first 10 attempts
+  // - then increase by 1s per attempt, up to a max of 30s
   const attempts = useRef(0);
 
   useEffect(() => {
-    let shouldContinue = true;
-    function continueCalling() {
-      const backoff = attempts.current < 10 ? 1000 : Math.min(30000, 1000 + (attempts.current - 10) * 1000);
-      setTimeout(() => {
-        if (!shouldContinue) {
-          return;
-        }
-        callback().then();
-        attempts.current++;
-        shouldContinue && continueCalling();
-      }, backoff);
-    }
+    let shouldContinue = true;
+    let timer: ReturnType<typeof setTimeout> | undefined;
+    const computeBackoff = () =>
+      attempts.current < 10 ? 1000 : Math.min(30000, 1000 + (attempts.current - 9) * 1000); // 10th -> 2s
+    const scheduleNext = (delay: number) => {
+      timer = setTimeout(async () => {
+        if (!shouldContinue) return;
+        try {
+          await callback();
+        } catch {
+          // keep polling even if one iteration fails
+        } finally {
+          attempts.current++;
+          if (shouldContinue) {
+            scheduleNext(computeBackoff());
+          }
+        }
+      }, delay);
+    };
 
-    continueCalling();
+    scheduleNext(1000);
 
     return () => {
       shouldContinue = false;
+      if (timer) {
+        clearTimeout(timer);
+      }
     };
   }, [callback]);
 }
♻️ Duplicate comments (3)
src/features/processEnd/feedback/Feedback.tsx (1)

13-20: Avoid stale previousData by comparing via a ref; drop deep dep to reduce effect churn

Current code closes over previousData and re-memos on a deep property, which can still race under async polling. Track the latest value in a ref and read from it inside the callback. This also simplifies dependencies.

@@
-  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const previousDataRef = useRef(previousData);
+  useEffect(() => {
+    previousDataRef.current = previousData;
+  }, [previousData]);
@@
-      result.data.currentTask.elementId !== previousData?.currentTask?.elementId
+      result.data.currentTask.elementId !== previousDataRef.current?.currentTask?.elementId
@@
-    optimisticallyUpdateProcess,
-    previousData?.currentTask?.elementId,
+    optimisticallyUpdateProcess,
+    // previousData is read from previousDataRef to avoid stale captures

Also applies to: 31-35, 42-48

src/core/routing/useIsNavigating.ts (1)

1-10: Return actual router navigation state; drop brittle hash check

Current logic inverts idle and couples to window.location.hash. Use react-router’s state directly.

Apply this diff:

-import { useLocation, useNavigation } from 'react-router-dom';
+import { useNavigation } from 'react-router-dom';

 export function useIsNavigating() {
-  const isIdle = useNavigation().state === 'idle';
-  const location = useLocation();
-  const expectedLocation = `${location.pathname}${location.search}`.replace(/^\//, '');
-  const locationIsUpToDate = window.location.hash.endsWith(expectedLocation);
-
-  return !locationIsUpToDate || !isIdle;
+  const { state } = useNavigation();
+  return state !== 'idle'; // true while submitting/loading
 }
src/components/wrappers/ProcessWrapper.tsx (1)

109-112: Archived→receipt loader is OK (navigation handled elsewhere)

Acknowledging the intentional design per project guidance; no change needed.

🧹 Nitpick comments (2)
src/features/datamodel/DataModelsProvider.tsx (2)

173-175: Keep the isFetching guard; add an explanatory comment to prevent regressions

Given instanceless apps disable the query, the guard must remain on isFetching (not success). Add a clarifying comment.

   // Find all data types referenced in dataModelBindings in the layout
   useEffect(() => {
-    if (isFetching) {
+    // Important: For instanceless apps, useInstanceDataQuery is disabled.
+    // We still need to parse referenced data types in that case.
+    // Only bail while actively fetching for instance-based apps.
+    if (isFetching) {
       return;
     }

152-160: Avoid double useApplicationMetadata() call; derive isStateless from the existing variable

Use the already-read applicationMetadata to prevent redundant context reads and ensure consistency if metadata updates.

   const applicationMetadata = useApplicationMetadata();
@@
-  const isStateless = useApplicationMetadata().isStatelessApp;
+  const isStateless = applicationMetadata.isStatelessApp;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca4439 and b555580.

📒 Files selected for processing (5)
  • src/components/wrappers/ProcessWrapper.tsx (5 hunks)
  • src/core/routing/useIsNavigating.ts (1 hunks)
  • src/features/datamodel/DataModelsProvider.tsx (5 hunks)
  • src/features/processEnd/feedback/Feedback.tsx (3 hunks)
  • src/hooks/useWaitForQueries.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options via queryOptions

Files:

  • src/hooks/useWaitForQueries.ts
  • src/features/processEnd/feedback/Feedback.tsx
  • src/components/wrappers/ProcessWrapper.tsx
  • src/core/routing/useIsNavigating.ts
  • src/features/datamodel/DataModelsProvider.tsx
🧠 Learnings (5)
📚 Learning: 2025-08-28T12:25:27.289Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/components/wrappers/ProcessWrapper.tsx:107-110
Timestamp: 2025-08-28T12:25:27.289Z
Learning: In ProcessWrapper.tsx, when taskType === ProcessTaskType.Archived and taskId !== TaskKeys.CustomReceipt, the component shows a loader with reason 'redirect-to-receipt' but does not perform explicit navigation. This is intentional - other parts of the system handle the actual navigation closer to where user interactions occur.

Applied to files:

  • src/features/processEnd/feedback/Feedback.tsx
  • src/components/wrappers/ProcessWrapper.tsx
📚 Learning: 2025-08-28T12:25:27.289Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/components/wrappers/ProcessWrapper.tsx:107-110
Timestamp: 2025-08-28T12:25:27.289Z
Learning: In the app-frontend-react codebase, the architecture favors navigation happening closer to user interactions or specific process state changes, rather than using universal effects that automatically redirect users. This is part of a design philosophy to avoid "free-standing effects that throw you around" and make navigation more predictable and contextual.

Applied to files:

  • src/components/wrappers/ProcessWrapper.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In DataModelsProvider.tsx, the effect that processes referenced data types must run for both stateless and non-stateless apps. The `isFetching` guard is appropriate because stateless apps don't fetch instance data (useInstanceDataQuery is disabled with `enabled: !isStateless`) but still need the data type processing to occur.

Applied to files:

  • src/features/datamodel/DataModelsProvider.tsx
📚 Learning: 2025-08-28T12:00:32.967Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/datamodel/DataModelsProvider.tsx:172-175
Timestamp: 2025-08-28T12:00:32.967Z
Learning: In the Altinn app-frontend-react codebase, "stateless" refers to "instanceless" applications. While stateless/instanceless apps don't have server-side instance data, they still require data models to be loaded and processed for local state management.

Applied to files:

  • src/features/datamodel/DataModelsProvider.tsx
📚 Learning: 2025-08-22T13:53:28.252Z
Learnt from: CR
PR: Altinn/app-frontend-react#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-22T13:53:28.252Z
Learning: Applies to **/*.test.{ts,tsx} : In tests, use `renderWithProviders` from `src/test/renderWithProviders.tsx` to supply required form layout context

Applied to files:

  • src/features/datamodel/DataModelsProvider.tsx
🧬 Code graph analysis (3)
src/features/processEnd/feedback/Feedback.tsx (3)
src/features/instance/useProcessQuery.ts (2)
  • useProcessQuery (23-26)
  • useOptimisticallyUpdateProcess (99-106)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
src/components/wrappers/ProcessWrapper.tsx (4)
src/core/routing/useIsNavigating.ts (1)
  • useIsNavigating (3-10)
src/hooks/useNavigatePage.ts (1)
  • useIsValidTaskId (184-202)
src/features/instance/useProcessQuery.ts (2)
  • useGetTaskTypeById (61-89)
  • useProcessQuery (23-26)
src/hooks/useWaitForQueries.ts (1)
  • useWaitForQueries (11-31)
src/features/datamodel/DataModelsProvider.tsx (2)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (123-132)
src/features/datamodel/utils.ts (2)
  • MissingDataElementException (27-36)
  • isDataTypeWritable (104-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Install
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
src/features/processEnd/feedback/Feedback.tsx (1)

21-36: Good: ended-first check prevents transient misnavigation

Prioritizing result.data.ended before task-change comparison is correct and avoids routing to a transient task after process end.

src/features/datamodel/DataModelsProvider.tsx (5)

4-4: Good: Blocking UI on in-flight saves with useMutationState

Import looks right and is used to prevent racey initial writes in BlockUntilLoaded.


30-30: Good: Consolidated instance data access via InstanceContext hooks

Switching to useInstanceDataQuery/useInstanceDataElements simplifies data sourcing and typing across the file.


160-163: Tuple-select for dataElements is precise and lightweight

The enabled: !isStateless gate and as const tuple shape work well with downstream utils like isDataTypeWritable.


196-201: Correct: Only require instance data elements for stateful apps

The non-stateless check avoids false positives for instanceless flows; logErrorOnce keeps noise low.


204-206: Writable detection aligned with new dataElements shape

Using isDataTypeWritable(dataType, isStateless, dataElements ?? []) matches the tuple refactor and preserves stateless semantics.

@olemartinorg olemartinorg changed the base branch from main to chore/local-cypress-fixes September 3, 2025 12:17
@olemartinorg olemartinorg moved this to 🔎 In review in Team Altinn Studio Sep 3, 2025
Base automatically changed from chore/local-cypress-fixes to main September 11, 2025 11:12
Ole Martin Handeland added 18 commits September 18, 2025 12:27
…e() instead. Moving things around to make tests/mocking work.
… same as for every other form page, and can be rendered out more easily in ProcessWrapper.tsx
… check and fix wrong navigation in FixWrongReceiptType.tsx.
…e data is not yet fetched again when we arrive at the DataModelsProvider. This causes it to use outdated instance data, marking the data model as read-only when it has been unlocked. This should fix the problem, and hopefully also solve the earlier bug with subforms.
…ithout flickering this message when navigating.
…/>, so it depended on having its own routing. Baking what's left into FixWrongReceiptType.tsx and removing everything that makes it look like CustomReceipt isn't just any other taskId
…avigating you to the first page. Updating the hook to make it work without a current location as well.
Ole Martin Handeland added 3 commits September 18, 2025 12:28
…cause network takes time the navigation hadn't happened quite yet. Waiting for these to finish before showing this message to the user fixes things.
…) needs info about pages that are hidden, and so it needs to run expressions. In one case in the ttd/payment-test app, when navigating to process/next, the FormDataWrite provider is no longer provided when useNavigatePage() is called, so everything crashes. This uses the lax selector instead, making things not crash anymore.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/features/processEnd/feedback/Feedback.tsx (1)

63-90: Prevent overlapping polls, fix off‑by‑one backoff, handle errors, and clear timer on unmount.

Current loop can schedule the next tick before the prior async finishes (parallel refetches), the delay is 1s for 11 ticks (off by one vs comment), errors are unhandled, and the timer isn’t cleared on cleanup.

-function useBackoff(callback: () => Promise<void>) {
+function useBackoff(callback: () => Promise<void>) {
   // The backoff algorithm is used to check the process data, and slow down the requests after a while.
   // At first, it starts off once a second (every 1000ms) for 10 seconds.
   // After that, it slows down by one more second for every request.
   // Once it reaches 20 attempts, it will reach the max delay of 30 seconds between each request.
-  const attempts = useRef(0);
+  const attempts = useRef(0);
+  const timeoutIdRef = useRef<number | undefined>(undefined);
@@
   useEffect(() => {
     let shouldContinue = true;
     function continueCalling() {
-      const backoff = attempts.current < 10 ? 1000 : Math.min(30000, 1000 + (attempts.current - 10) * 1000);
-      setTimeout(() => {
+      // After 10 attempts at 1s, increase by 1s per attempt, capped at 30s.
+      const backoff =
+        attempts.current < 10 ? 1000 : Math.min(30000, 1000 + (attempts.current - 9) * 1000);
+      timeoutIdRef.current = window.setTimeout(() => {
         if (!shouldContinue) {
           return;
         }
-        callback().then();
-        attempts.current++;
-        shouldContinue && continueCalling();
+        void callback()
+          .catch(() => {
+            // swallow or add debug log; avoid unhandled rejections
+          })
+          .finally(() => {
+            attempts.current++;
+            if (shouldContinue) {
+              continueCalling();
+            }
+          });
-      }, backoff);
+      }, backoff);
     }
 
     continueCalling();
 
     return () => {
       shouldContinue = false;
+      if (timeoutIdRef.current !== undefined) {
+        clearTimeout(timeoutIdRef.current);
+      }
     };
   }, [callback]);
 }
♻️ Duplicate comments (4)
src/components/wrappers/ProcessWrapper.tsx (2)

109-111: Archived task shows loader without explicit redirect — intentional

Acknowledging prior decision: redirect handled elsewhere; keeping this as a passive loader is by design.


197-231: Annotate hook return type (nit) and keep semantics explicit

The hook returns boolean | null; make it explicit.

-function useIsWrongTask(taskId: string | undefined) {
+function useIsWrongTask(taskId: string | undefined): boolean | null {

Note: prior discussions covered timeout cleanup/abort; not rehashing here.

src/features/processEnd/feedback/Feedback.tsx (2)

27-35: Ended-first navigation ordering is correct.

Thanks for prioritizing ended before task-change; this prevents transient misnavigation.


27-35: Decouple polling restarts from query cache churn; read previous process via ref.

Using previousData?.currentTask?.elementId in deps causes the backoff loop to restart on every cache change. Keep a ref of the latest process to compare inside the callback and remove it from deps.

-  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const { refetch: reFetchProcessData, data: previousData } = useProcessQuery();
+  const previousDataRef = useRef(previousData);
+  useEffect(() => {
+    previousDataRef.current = previousData;
+  }, [previousData]);
@@
-    } else if (
-      result.data.currentTask?.elementId &&
-      result.data.currentTask.elementId !== previousData?.currentTask?.elementId
-    ) {
+    } else if (
+      result.data.currentTask?.elementId &&
+      result.data.currentTask.elementId !== previousDataRef.current?.currentTask?.elementId
+    ) {
       navigateTo = result.data.currentTask.elementId;
     }
@@
   }, [
     navigateToTask,
     optimisticallyUpdateProcess,
-    previousData?.currentTask?.elementId,
     reFetchInstanceData,
     reFetchProcessData,
   ]);

Also applies to: 42-48

🧹 Nitpick comments (8)
src/components/wrappers/ProcessWrapper.tsx (2)

78-81: Guard navigate while router is busy; normalize isMutating to boolean

Good guard with isNavigating. Minor nit: coerce isMutating count to boolean for consistency with helpers.

-  const isRunningProcessNext = queryClient.isMutating({ mutationKey: processNextKey });
+  const isRunningProcessNext = queryClient.isMutating({ mutationKey: processNextKey }) > 0;

Also applies to: 86-89


179-195: Add explicit return types for helpers (nit)

Tiny clarity win; no behavior change.

-function isRunningProcessNext(queryClient: QueryClient) {
+function isRunningProcessNext(queryClient: QueryClient): boolean {
   return queryClient.isMutating({ mutationKey: getProcessNextMutationKey() }) > 0;
 }
 
-function useIsRunningProcessNext() {
+function useIsRunningProcessNext(): boolean | null {
   const queryClient = useQueryClient();
   const [isMutating, setIsMutating] = useState<boolean | null>(null);
src/features/processEnd/feedback/Feedback.tsx (1)

13-20: Good: disabled instance query for explicit refetch; also disable focus-refetch.

Nice switch to enabled: false. Consider also disabling focus refetch to avoid accidental fetches.

-  const reFetchInstanceData = useInstanceDataQuery({ enabled: false }).refetch;
+  const reFetchInstanceData = useInstanceDataQuery({ enabled: false, refetchOnWindowFocus: false }).refetch;
src/features/propagateTraceWhenPdf/index.ts (4)

6-13: Type the cookie map and harden parsing (avoid empty keys, prototype keys).

Prevents TS index errors and edge cases when document.cookie is empty.

Apply:

-function getCookies(): { [key: string]: string } {
-  const cookie = {};
-  document.cookie.split(';').forEach((el) => {
-    const split = el.split('=');
-    cookie[split[0].trim()] = split.slice(1).join('=');
-  });
-  return cookie;
-}
+function getCookies(): Record<string, string> {
+  const cookies: Record<string, string> = Object.create(null);
+  const raw = typeof document !== 'undefined' ? document.cookie : '';
+  if (!raw) return cookies;
+  raw.split(';').forEach((el) => {
+    if (!el) return;
+    const [rawKey, ...rest] = el.split('=');
+    const key = rawKey?.trim();
+    if (!key) return;
+    cookies[key] = rest.join('=');
+  });
+  return cookies;
+}

15-20: Guard against SSR/non‑browser invocation.

Avoids ReferenceErrors if called outside the browser (tests/SSR).

 export function propagateTraceWhenPdf() {
   try {
-    const hash = window.location.hash;
+    if (typeof window === 'undefined' || typeof document === 'undefined') return;
+    const hash = window.location.hash;
     const search = hash.split('?')[1] ?? '';
     const isPdf = new URLSearchParams(search).get(SearchParams.Pdf) === '1';

4-6: Prevent duplicate axios interceptors on repeated calls.

Without gating/ejecting, repeated invocations can stack interceptors.

 import { appPath } from 'src/utils/urls/appUrlHelper';
 
+let pdfInterceptorId: number | null = null;
+
 function getCookies(): { [key: string]: string } {
-    if (isPdf) {
+    if (isPdf && pdfInterceptorId === null) {
       const cookies = getCookies();
-      axios.interceptors.request.use((config) => {
+      pdfInterceptorId = axios.interceptors.request.use((config) => {
         try {
           if (config.url?.startsWith(appPath) !== true) {
             return config;
           }

Optionally return a disposer from this function (or handle elsewhere) to eject when no longer needed:

// elsewhere when appropriate:
// if (pdfInterceptorId !== null) { axios.interceptors.request.eject(pdfInterceptorId); pdfInterceptorId = null; }

Also applies to: 21-46


29-39: Header assignment: be compatible with Axios v1 types.

Use AxiosHeaders.set when available; otherwise fall back to plain object merge.

-          // This header is caught in app-lib/backend and used to allow injection of traceparent/context
-          config.headers['X-Altinn-IsPdf'] = 'true';
-
-          const traceparent = cookies['altinn-telemetry-traceparent'];
-          const tracestate = cookies['altinn-telemetry-tracestate'];
-          if (traceparent) {
-            config.headers['traceparent'] = traceparent;
-          }
-          if (tracestate) {
-            config.headers['tracestate'] = tracestate;
-          }
+          // This header is caught in app-lib/backend and used to allow injection of traceparent/context
+          const traceparent = cookies['altinn-telemetry-traceparent'];
+          const tracestate = cookies['altinn-telemetry-tracestate'];
+          const h = config.headers as unknown;
+          if (h && typeof (h as { set?: Function }).set === 'function') {
+            (h as { set: (k: string, v: string) => void }).set('X-Altinn-IsPdf', 'true');
+            if (traceparent) (h as { set: (k: string, v: string) => void }).set('traceparent', traceparent);
+            if (tracestate) (h as { set: (k: string, v: string) => void }).set('tracestate', tracestate);
+          } else {
+            config.headers = {
+              ...(config.headers ?? {}),
+              'X-Altinn-IsPdf': 'true',
+              ...(traceparent ? { traceparent } : {}),
+              ...(tracestate ? { tracestate } : {}),
+            };
+          }
src/features/receipt/ReceiptContainer.tsx (1)

78-82: Redundant prop: PresentationComponent already hides navigation on receipt pages.

PresentationComponent derives isReceipt internally and sets showNavigation to false for receipts. The explicit showNavigation={false} here is unnecessary.

Apply this diff if you keep this block:

-      <PresentationComponent showNavigation={false}>
+      <PresentationComponent>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5934d28 and d3d6e69.

📒 Files selected for processing (37)
  • src/App.tsx (4 hunks)
  • src/components/form/Form.tsx (2 hunks)
  • src/components/form/LinkToPotentialNode.tsx (1 hunks)
  • src/components/presentation/Presentation.test.tsx (6 hunks)
  • src/components/presentation/Presentation.tsx (5 hunks)
  • src/components/wrappers/ProcessWrapper.tsx (5 hunks)
  • src/core/loading/Loader.tsx (0 hunks)
  • src/core/routing/types.ts (1 hunks)
  • src/core/routing/useIsNavigating.ts (1 hunks)
  • src/core/routing/useIsReceiptPage.ts (1 hunks)
  • src/features/datamodel/DataModelsProvider.tsx (5 hunks)
  • src/features/datamodel/utils.ts (1 hunks)
  • src/features/expressions/expression-functions.ts (2 hunks)
  • src/features/formData/FormDataWrite.tsx (2 hunks)
  • src/features/instance/useProcessNext.tsx (2 hunks)
  • src/features/instance/useProcessQuery.ts (2 hunks)
  • src/features/instantiate/selection/InstanceSelection.tsx (1 hunks)
  • src/features/navigation/AppNavigation.tsx (2 hunks)
  • src/features/navigation/utils.ts (1 hunks)
  • src/features/pdf/PDFWrapper.test.tsx (1 hunks)
  • src/features/processEnd/feedback/Feedback.tsx (3 hunks)
  • src/features/propagateTraceWhenPdf/index.ts (1 hunks)
  • src/features/receipt/FixWrongReceiptType.tsx (1 hunks)
  • src/features/receipt/ReceiptContainer.tsx (2 hunks)
  • src/hooks/navigation.ts (1 hunks)
  • src/hooks/useIsPdf.ts (1 hunks)
  • src/hooks/useNavigatePage.ts (2 hunks)
  • src/hooks/useWaitForQueries.ts (1 hunks)
  • src/layout/GenericComponent.tsx (2 hunks)
  • src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx (1 hunks)
  • src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx (1 hunks)
  • src/layout/Subform/SubformWrapper.tsx (1 hunks)
  • src/layout/Tabs/Tabs.tsx (1 hunks)
  • src/layout/index.ts (2 hunks)
  • src/utils/layout/useExpressionDataSources.ts (3 hunks)
  • test/e2e/integration/stateless-app/feedback.ts (1 hunks)
  • test/e2e/integration/stateless-app/receipt.ts (2 hunks)
💤 Files with no reviewable changes (1)
  • src/core/loading/Loader.tsx
✅ Files skipped from review due to trivial changes (2)
  • src/features/navigation/AppNavigation.tsx
  • src/layout/Tabs/Tabs.tsx
🚧 Files skipped from review as they are similar to previous changes (30)
  • src/core/routing/types.ts
  • src/components/form/LinkToPotentialNode.tsx
  • src/layout/RepeatingGroup/Providers/RepeatingGroupFocusContext.tsx
  • src/core/routing/useIsReceiptPage.ts
  • src/features/expressions/expression-functions.ts
  • src/layout/RepeatingGroup/EditContainer/RepeatingGroupEditContext.tsx
  • src/layout/Subform/SubformWrapper.tsx
  • src/hooks/useIsPdf.ts
  • src/features/navigation/utils.ts
  • src/features/receipt/FixWrongReceiptType.tsx
  • src/features/instance/useProcessNext.tsx
  • src/hooks/useNavigatePage.ts
  • src/core/routing/useIsNavigating.ts
  • src/features/pdf/PDFWrapper.test.tsx
  • src/layout/index.ts
  • src/components/form/Form.tsx
  • src/layout/GenericComponent.tsx
  • src/components/presentation/Presentation.tsx
  • src/utils/layout/useExpressionDataSources.ts
  • src/components/presentation/Presentation.test.tsx
  • test/e2e/integration/stateless-app/receipt.ts
  • src/features/formData/FormDataWrite.tsx
  • src/features/datamodel/utils.ts
  • src/features/instance/useProcessQuery.ts
  • src/hooks/navigation.ts
  • src/features/instantiate/selection/InstanceSelection.tsx
  • src/App.tsx
  • src/hooks/useWaitForQueries.ts
  • test/e2e/integration/stateless-app/feedback.ts
  • src/features/datamodel/DataModelsProvider.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Avoid using any and unnecessary type casts (as Type) in TypeScript; prefer precise typings and refactor existing casts/anys
For TanStack Query, use objects to manage query keys and functions, and centralize shared options via queryOptions

Files:

  • src/features/propagateTraceWhenPdf/index.ts
  • src/features/receipt/ReceiptContainer.tsx
  • src/features/processEnd/feedback/Feedback.tsx
  • src/components/wrappers/ProcessWrapper.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/receipt/FixWrongReceiptType.tsx:16-34
Timestamp: 2025-09-03T14:23:33.606Z
Learning: In FixWrongReceiptType.tsx, brief ping-ponging between receipt routes during instance data loading is acceptable because the instance data is typically already fetched when this component runs. The author considers this a non-issue.
📚 Learning: 2025-09-03T14:23:33.606Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/features/receipt/FixWrongReceiptType.tsx:16-34
Timestamp: 2025-09-03T14:23:33.606Z
Learning: In FixWrongReceiptType.tsx, brief ping-ponging between receipt routes during instance data loading is acceptable because the instance data is typically already fetched when this component runs. The author considers this a non-issue.

Applied to files:

  • src/features/receipt/ReceiptContainer.tsx
  • src/components/wrappers/ProcessWrapper.tsx
📚 Learning: 2025-08-28T12:25:27.289Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/components/wrappers/ProcessWrapper.tsx:107-110
Timestamp: 2025-08-28T12:25:27.289Z
Learning: In ProcessWrapper.tsx, when taskType === ProcessTaskType.Archived and taskId !== TaskKeys.CustomReceipt, the component shows a loader with reason 'redirect-to-receipt' but does not perform explicit navigation. This is intentional - other parts of the system handle the actual navigation closer to where user interactions occur.

Applied to files:

  • src/features/receipt/ReceiptContainer.tsx
  • src/features/processEnd/feedback/Feedback.tsx
  • src/components/wrappers/ProcessWrapper.tsx
📚 Learning: 2025-09-03T14:26:18.627Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/components/wrappers/ProcessWrapper.tsx:78-83
Timestamp: 2025-09-03T14:26:18.627Z
Learning: In ProcessWrapper.tsx, the useIsRunningProcessNext() hook intentionally uses a non-reactive pattern with queryClient.isMutating and local state instead of useIsMutating hook. This design choice is deliberate and should not be changed to a reactive pattern.

Applied to files:

  • src/features/processEnd/feedback/Feedback.tsx
  • src/components/wrappers/ProcessWrapper.tsx
📚 Learning: 2025-08-28T12:25:27.289Z
Learnt from: olemartinorg
PR: Altinn/app-frontend-react#3645
File: src/components/wrappers/ProcessWrapper.tsx:107-110
Timestamp: 2025-08-28T12:25:27.289Z
Learning: In the app-frontend-react codebase, the architecture favors navigation happening closer to user interactions or specific process state changes, rather than using universal effects that automatically redirect users. This is part of a design philosophy to avoid "free-standing effects that throw you around" and make navigation more predictable and contextual.

Applied to files:

  • src/components/wrappers/ProcessWrapper.tsx
🧬 Code graph analysis (3)
src/features/receipt/ReceiptContainer.tsx (2)
src/features/receipt/FixWrongReceiptType.tsx (1)
  • FixWrongReceiptType (16-47)
src/components/presentation/Presentation.tsx (1)
  • PresentationComponent (32-93)
src/features/processEnd/feedback/Feedback.tsx (3)
src/features/instance/useProcessQuery.ts (2)
  • useProcessQuery (23-26)
  • useOptimisticallyUpdateProcess (99-106)
src/hooks/useNavigatePage.ts (1)
  • useNavigateToTask (152-182)
src/features/instance/InstanceContext.tsx (1)
  • useInstanceDataQuery (110-119)
src/components/wrappers/ProcessWrapper.tsx (4)
src/core/routing/useIsNavigating.ts (1)
  • useIsNavigating (3-10)
src/hooks/useNavigatePage.ts (1)
  • useIsValidTaskId (184-202)
src/features/instance/useProcessQuery.ts (2)
  • useGetTaskTypeById (61-89)
  • useProcessQuery (23-26)
src/hooks/useWaitForQueries.ts (1)
  • useWaitForQueries (11-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Install
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Type-checks, eslint, unit tests and SonarCloud
🔇 Additional comments (6)
src/components/wrappers/ProcessWrapper.tsx (4)

6-6: Imports: type-only + routing/query hooks — LGTM

Clean type-only import and the added hooks match usage below.

Also applies to: 13-13, 24-24


100-106: Null-state gating to avoid flicker — LGTM

Deferring render until both gates resolve is appropriate.


116-119: Disable navigation on error pages — LGTM

showNavigation={false} fits the flow for invalid/wrong task states.

Also applies to: 124-127


132-135: Wrap Confirm/Feedback in PresentationComponent — LGTM

Matches the refactor to centralize receipt/state logic.

Also applies to: 140-143

src/features/processEnd/feedback/Feedback.tsx (1)

50-52: Separation of concerns looks good.

Polling is isolated behind useBackoff(callback); once navigation unmounts the page, cleanup halts polling. No action needed.

src/features/propagateTraceWhenPdf/index.ts (1)

3-3: Import move looks correct — unable to verify repo-wide (rg skipped files).

Repository search returned no files; cannot confirm there are no remaining imports of SearchParams. Run locally to verify:
rg -n --hidden -uu -S "\bSearchParams\b" -g '!node_modules/**' -C1

@ErlingHauan ErlingHauan self-assigned this Oct 1, 2025
@ErlingHauan ErlingHauan moved this to 🔎 In review in Team Altinn Studio Oct 1, 2025
@ErlingHauan ErlingHauan added the squad/flyt Issues that belongs to the named squad. label Oct 1, 2025
Copy link

@ErlingHauan ErlingHauan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! The changes look sensible, with a disclaimer that I'm not familiar with this repo.
I've added a nitpick comment below, but it might be a subjective opinion only, so I'm approving this PR.

# Conflicts:
#	src/features/instantiate/selection/InstanceSelection.tsx
Copy link

sonarqubecloud bot commented Oct 1, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
37.7% Coverage on New Code (required ≥ 45%)
24.2% Condition Coverage on New Code (required ≥ 45%)

See analysis details on SonarQube Cloud

@olemartinorg olemartinorg merged commit af24842 into main Oct 1, 2025
14 of 16 checks passed
@olemartinorg olemartinorg deleted the bug/feedback-step branch October 1, 2025 14:41
@github-project-automation github-project-automation bot moved this from 🔎 In review to ✅ Done in Team Altinn Studio Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-ignore This PR is a new feature and should not be cherry-picked onto release branches kind/bug Something isn't working squad/flyt Issues that belongs to the named squad.
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Feedback step only recognizes EndEvent
3 participants